Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(@angular/cli): error out when command json is invalid #12554

Merged
merged 1 commit into from
Nov 1, 2018
Merged

fix(@angular/cli): error out when command json is invalid #12554

merged 1 commit into from
Nov 1, 2018

Conversation

filipesilva
Copy link
Contributor

petebacondarwin
petebacondarwin reviewed Oct 11, 2018
View reviewed changes
packages/angular/cli/commands/e2e.json Outdated
@@ -2,7 +2,7 @@
"$schema": "http://json-schema.org/schema",
"$id": "ng-cli://commands/e2e.json",
"description": "Builds and serves an Angular app, and runs end-to-end tests using Protractor.",
"$longDescription": "Must be executed from within a workspace directory.\n When a project name is not supplied, the configured default e2e project of the workspace is used.",
"$longDescription": "./e2e-long.md",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So was this the error that caused the output to be invalid?

Copy link
Contributor Author

@filipesilva filipesilva Oct 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that was the one. That should be a file path. We output a warning instead of failing before.

The warning would actually happen on every ng command, not just the help ones. You can see that in https://circleci.com/gh/angular/angular-cli/22079:

Running `ng "version"`...
CWD: /tmp/angular-cli-e2e-118910-90-1pz8kw8.dr34h
ENV: undefined
  File /home/circleci/.npm-global/lib/node_modules/@angular/cli/commands/Must be executed from within a workspace directory.
   When a project name is not supplied, the configured default e2e project of the workspace is used. was not found while constructing the subcommand e2e.
       _                      _                 ____ _     ___
      / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
     / △ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
    / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
   /_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
                  |___/
      
  Angular CLI: 7.0.0-rc.2
  Node: 10.9.0
  OS: linux x64
  Angular: 
  ... 
  Package                      Version
  ------------------------------------------------------
  @angular-devkit/architect    0.9.0-rc.2
  @angular-devkit/core         7.0.0-rc.2
  @angular-devkit/schematics   7.0.0-rc.2
  @schematics/angular          7.0.0-rc.2
  @schematics/update           0.9.0-rc.2
  rxjs                         6.3.3
  typescript                   3.1.1

@filipesilva filipesilva force-pushed the command-json-path-error branch 2 times, most recently from 5d51689 to 027f003 Compare October 11, 2018 12:12
petebacondarwin
petebacondarwin reviewed Oct 11, 2018
View reviewed changes
scripts/snapshots.ts
fs.writeFileSync(path.join(helpOutputRoot, commandName + '.json'), stdout);
}
// Make sure the output is JSON before printing it, and format it as well.
const jsonOutput = JSON.stringify(JSON.parse(stdout.trim()), undefined, 2);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😻

filipesilva
filipesilva commented Oct 11, 2018
View reviewed changes
scripts/snapshots.ts
}
// Make sure the output is JSON before printing it, and format it as well.
const jsonOutput = JSON.stringify(JSON.parse(stdout.trim()), undefined, 2);
fs.writeFileSync(path.join(helpOutputRoot, commandName + '.json'), jsonOutput);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like the kind of thing that should be part of build, instead of snapshot.

@hansl @clydin is there a reason it's not there?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build does not generate the json files. We don’t publish those files to npm.

@ngbot
Copy link

ngbot bot commented Oct 11, 2018

Artifact Baseline Current Change
❕ cli/new-production/test-project/main.js 169.01KB 169.11KB +104 bytes

1 similar comment
@ngbot
Copy link

ngbot bot commented Oct 11, 2018

Artifact Baseline Current Change
❕ cli/new-production/test-project/main.js 169.01KB 169.11KB +104 bytes

@filipesilva filipesilva mentioned this pull request Oct 11, 2018
Merged
@ngbot
Copy link

ngbot bot commented Oct 23, 2018

Hi @filipesilva! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

@filipesilva filipesilva added target: patch This PR is targeted for the next patch release and removed state: blocked labels Oct 24, 2018
@alexeagle alexeagle removed the request for review from clydin November 1, 2018 15:25
@alexeagle alexeagle merged commit eef66f0 into angular:master Nov 1, 2018
@filipesilva filipesilva deleted the command-json-path-error branch November 1, 2018 15:59
filipesilva added a commit to filipesilva/angular-cli that referenced this pull request Nov 1, 2018
@filipesilva
test(@angular/cli): fix implicit import
854b5bb 
This lint error resulted from the interaction between angular#12554 and angular#12515.
@filipesilva filipesilva mentioned this pull request Nov 1, 2018
Merged
alexeagle pushed a commit that referenced this pull request Nov 1, 2018
@filipesilva @alexeagle
test(@angular/cli): fix implicit import
298faa1 
This lint error resulted from the interaction between #12554 and #12515.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@petebacondarwin petebacondarwin petebacondarwin left review comments

@hansl hansl hansl approved these changes

Assignees
No one assigned
Labels
target: patch This PR is targeted for the next patch release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@filipesilva @petebacondarwin @hansl @alexeagle @googlebot